Skip to content

Full project refactor: dead-code purge + Kotlin/Compose modernization + Rust JNI hardening + build tooling#45

Merged
madeye merged 14 commits into
masterfrom
refactor/full-cleanup
May 12, 2026
Merged

Full project refactor: dead-code purge + Kotlin/Compose modernization + Rust JNI hardening + build tooling#45
madeye merged 14 commits into
masterfrom
refactor/full-cleanup

Conversation

@madeye
Copy link
Copy Markdown
Owner

@madeye madeye commented May 12, 2026

Multi-agent refactor of ProxyDroid. Orchestrated by a 5-agent team (1 inventory + 4 parallel workers), one merge commit per worker on top of master. Verified locally with ./gradlew :app:assembleDebug + scripts/run_local_emulator_tests.sh8/8 e2e tests green.

Headline numbers

174 files changed, +1,308 / −55,632. ~33% Kotlin LOC reduction; entire libevent + redsocks C subtrees gone.

What changed, by worker

1. Dead-code purge (refactor/dead-code-purge, 3 commits)

  • Removed app/src/main/cpp/libevent/ (85 files, 45,459 LOC) and app/src/main/cpp/redsocks/ (34 files, ~7,000 LOC) — both obsoleted by the VPN-first redesign.
  • Removed the root/iptables Exec JNI shim (cpp/exec/, Exec.kt) and the dead Kotlin helpers in Utils.kt (checkRoot, runRootCommand, runScript, isRoot, etc.).
  • Removed pre-API-24 drawable variants (drawable-{hdpi,ldpi,mdpi,xhdpi}-v{9,11}/), unused button_*.xml selectors.
  • Reduced cpp/CMakeLists.txt to a single placeholder target so AGP's externalNativeBuild still configures.

2. Kotlin / Compose modernization (refactor/kotlin-modernize, 3 commits)

  • Kotlin LOC 4170 → 2803 (−1367, −33%); file count 36 → 20.
  • Deleted dead modules: com/btr/proxy/** (PAC engine), InnerSocketBuilder, ImageLoader[Factory], DomainValidator/RegexValidator, ProxyedApp.
  • Replaced Thread { }.start() / Handler(Looper.getMainLooper()) / a 1-second polling loop with coroutines + StateFlow throughout ProxyDroidVpnService, ConnectivityBroadcastReceiver, MainViewModel, ProxyDroid.
  • Tightened Profile.kt (dropped dead certificate; safe-call nullability), shrank Utils.kt to 68 LOC of pure VPN-state plumbing.

3. Rust crate cleanup (refactor/rust-cleanup, 3 commits)

  • cargo fmt + cargo clippy --target aarch64-linux-android -- -D warnings clean (0/0).
  • thiserror-based Tun2SocksError; expect() / String errors replaced with structured returns.
  • JNI boundary hardened with catch_unwind — panics no longer propagate across FFI (UB). nativeStart now returns -2 on panic, -1 on expected failure, 0 on success. Kotlin caller already checks rc != 0, so ABI is compatible.
  • Graceful shutdown via oneshot channel (replaces busy-polled AtomicBool).
  • Hardened unsafe fcntl calls (check for -1), dropped dead protected_connect.

4. Build / tooling (refactor/build-tooling, 5 commits)

  • Introduced gradle/libs.versions.toml as the single source of dep versions; regrouped app/build.gradle dependencies (AndroidX / Compose / Kotlin / test).
  • Tidied gradle.properties, kept the AGP-8.1.2 + android.suppressUnsupportedCompileSdk=36 block (deliberate — bumping AGP retriggers the rust-android-gradle 0.9.6 mergeJniLibFolders landmine).
  • .github/workflows/android-build.yml: cache catalog, drop dead Create dummy google-services.json step, add refactor/** branch filter.
  • README.md rewritten for VPN-first / Compose / Rust netstack-smoltcp reality (was still claiming redsocks + iptables).
  • privacy_policy.md: removed stale AdMob / Firebase references.

Items requested for human eyeball

  1. android-build.yml: confirm no downstream CI consumer expected the dummy google-services.json. Verified clean of GMS plugins/runtimes by the agent.
  2. privacy_policy.md: confirm wording for Play listing.
  3. Anything previously calling Utils.copyAssets / Utils.getDataPath — they were unreferenced and have been dropped.

Verification

  • ./gradlew :app:assembleDebug — green (after one-time app/.cxx cache wipe because the deleted cpp/exec was cached).
  • scripts/run_local_emulator_tests.sh on AVD meow_api35 (arm64-v8a, API 35): 8/8 tests green in 0.3s of test time.

🤖 Generated with Claude Code

madeye and others added 14 commits May 12, 2026 09:28
Both legacy components are obsolete after the VPN-first
netstack-smoltcp rewrite:
- libevent (45K LOC) was only consumed by redsocks
- redsocks (transparent iptables proxy) is unreachable from the
  Compose UI / VPN service path

No Kotlin/Java code references either tree. CMakeLists root no
longer descends into them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
minSdk is 24, so -v9 and -v11 resource qualifier buckets are dead.
The four ic_stat_proxydroid.png copies in each are superseded by
the unqualified drawable-{hdpi,ldpi,mdpi,xhdpi}/ variants still
referenced by ProxyDroidVpnService.

button_gray/white/selector.xml were only self-referential; no
layout or Compose code consumes them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the entire root command-execution surface that the
VPN-first architecture no longer needs:

- app/src/main/cpp/exec/termExec.cpp + CMakeLists.txt
  (createSubprocess/waitFor JNI bindings for su shell)
- app/src/main/java/org/proxydroid/Exec.kt
  (Kotlin companion to termExec.cpp)
- Utils.kt: checkRoot, runRootCommand, runScript,
  getHasRedirectSupport, getIptablesPath, getShellPath,
  isRoot field + setRoot/isRoot accessors

None of these have callers outside Utils.kt itself; the Compose
UI, MainViewModel, ProxyDroidVpnService, ConnectivityBroadcastReceiver
and ProxyDroidWidgetProvider only use Utils.isWorking/setWorking
and Utils.isConnecting/setConnecting, which are preserved.

cpp/CMakeLists.txt keeps a single placeholder.c target so AGP's
externalNativeBuild CMake configure step still has something to
build until app/build.gradle drops externalNativeBuild entirely
(owned by the build-tooling agent).

Build verified: ./gradlew :app:assembleDebug -> BUILD SUCCESSFUL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move all dependency and plugin version literals from build.gradle and
app/build.gradle into a standard Gradle version catalog. Group app
dependencies into androidx core / Kotlin / Compose / third-party / test
sections with brief comments.

AGP 8.1.2, kotlin 1.9.10, rust-android 0.9.6 and compose-compiler 1.5.3
are now declared in one place. No version bumps — pure refactor.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the commented-out org.gradle.parallel scaffold and verbose doc
boilerplate inherited from `android create-project`. Keep every active
setting documented; preserve the AGP 8.1.2 compileSdk-36 suppression
block verbatim per refactor brief.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…alog

- Remove the four 'Create dummy google-services.json' steps. The app has
  no Firebase/GMS plugin and no code path reads google-services.json;
  they were inherited scaffolding.
- Add 'refactor/**' to push + PR branch filters so the cleanup branches
  exercise CI.
- Include gradle/libs.versions.toml in the Gradle cache key so version
  catalog changes invalidate cache correctly.
- Cargo + Gradle caches already in place; left unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the legacy redsocks/iptables-era README. The current reality:

- VPN-only (no root, no iptables, no redsocks)
- Rust tun2socks via netstack-smoltcp wired through rust-android-gradle
- Jetpack Compose Material 3 UI
- minSdk 24, target/compile 36, AGP 8.1.2 pinned (with rationale)

PROJECT STRUCTURE section now matches the post-refactor tree
(cpp/exec only, rust/proxydroid-tun2socks, gradle/libs.versions.toml)
and flags the legacy libevent/redsocks dirs as scheduled for removal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The app ships no advertising, analytics, or crash-reporting SDKs — the
listed third-party services are stale claims. Replace that paragraph
with an accurate statement scoped to the VPN traffic itself.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…et2 dep

Introduce a structured error type for the JNI seam and the top-level
start/stop paths so we can stop relying on String errors and on
.expect() inside ABI-exported code. Lower-level I/O continues to use
std::io::Error.

Also drop the socket2 direct dependency: it was only referenced by the
dead protected_connect() helper (removed in a follow-up commit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- start() now returns Result<(), Tun2SocksError> instead of Result<(),
  String>; doh_client::init_doh_client returns the same.
- Remove .expect() calls on the netstack-smoltcp TCP runner/listener and
  the reqwest client builder. These ran on the runtime task, so a panic
  there would unwind into tokio and tear the JVM down. They are now
  io::Error / Tun2SocksError values.
- Add a oneshot shutdown channel so stop() can signal the runner to drop
  out of its main select{} rather than relying solely on the AtomicBool
  busy-poll. Previous behaviour (poll the flag in the TUN reader) is
  retained as a fallback.
- Guard the unsafe fcntl() calls: log on F_GETFL / F_SETFL failure
  instead of silently writing back -1 | O_NONBLOCK.
- Reset TUN2SOCKS_RUNNING on every early-return path inside start() so a
  failed start is retryable without restarting the process.

Behavioural change at the JNI seam: an invalid proxy URL now returns -1
from nativeStart synchronously instead of panicking inside the runtime
task after Kotlin has already moved on.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d_connect

- Wrap both Java_..._nativeStart and Java_..._nativeStop bodies in
  std::panic::catch_unwind so a panic in Rust can never unwind across
  the FFI boundary (which is undefined behaviour). nativeStart returns
  -2 on caught panic, -1 on expected error, 0 on success.
- Replace the panicking get_runtime() with try_get_runtime() that
  returns Tun2SocksError::Runtime on builder failure.
- Drop the dead protected_connect() helper from protect.rs. It was
  marked #[allow(dead_code)], constructed the same socket twice via
  socket2 (the first socket and its protect() call were leaked) and
  was never wired in — outbound proxy traffic relies on
  addDisallowedApplication() on the VpnService instead. protect_fd()
  is kept (still allow(dead_code)) so the relay can opt in later if
  that policy changes.
- Surface a JNI error if new_global_ref fails inside set_vpn_service
  rather than silently storing None.

ABI: the #[no_mangle] symbol names
Java_org_proxydroid_utils_Tun2SocksHelper_nativeStart and
Java_org_proxydroid_utils_Tun2SocksHelper_nativeStop are unchanged;
the C signatures match the existing Kotlin externals.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Compose VPN-first UI never reads the PAC engine (com.btr.proxy),
the legacy ImageLoader / ImageLoaderFactory thread pool, or
InnerSocketBuilder. Profile.certificate was persisted but never
rendered or forwarded to the netstack. Utils.kt also kept Runtime.exec
("su") root helpers and an iptables probe that no caller invokes
under the VPN-only flow.

Removes:
  - app/src/main/java/com/btr/proxy/** (PAC engine, 10 files, ~675 LOC)
  - InnerSocketBuilder.kt
  - utils/ImageLoader.kt + ImageLoaderFactory.kt
  - Profile.certificate field (storage + JSON round-trip)
  - Utils.runRootCommand / checkRoot / runScript / copyAssets /
    getHasRedirectSupport / getIptablesPath / getShellPath /
    isRoot / setRoot

Profile.kt: tightens nullability with .orEmpty(), drops mutable
`certificate` field, and removes the redundant try/catch around
toIntOrNull for port parsing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ProxyDroidVpnService now owns a SupervisorJob+Dispatchers.IO scope; the
worker thread that built the tun device and started the Rust tun2socks
is launched into that scope and cancelled in onDestroy. The
`vpnInterface!!` deref is replaced with a local non-null after
.establish().

ConnectivityBroadcastReceiver swaps Handler(Looper.getMainLooper()) +
handler.post for BroadcastReceiver.goAsync() backed by a process-wide
IO CoroutineScope. The handler body was split out and tightened with
early-exits.

Utils now exposes `working` and `connecting` as StateFlow<Boolean>
in addition to the legacy @JvmStatic getters. MainViewModel collects
those flows in viewModelScope, so the Compose connection card updates
reactively instead of polling once per second. The polling loop in
ProxyDroid.onCreate is gone.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DomainValidator + utils/RegexValidator have no inbound references
under the VPN-first Compose UI. The legacy AppManager.getProxyedApps
companion method was superseded by the local Compose loadApps() in
the same file; the ProxyedApp data class only existed to serve that
dead method.

Drops:
  - DomainValidator.kt
  - utils/RegexValidator.kt
  - ProxyedApp.kt
  - AppManager.getProxyedApps companion (plus the lone `username!!`)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@madeye madeye force-pushed the refactor/full-cleanup branch from 2e75bbf to 308c81b Compare May 12, 2026 06:00
@madeye madeye merged commit 69eec81 into master May 12, 2026
@madeye madeye deleted the refactor/full-cleanup branch May 12, 2026 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants